-
Notifications
You must be signed in to change notification settings - Fork 186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[draft] Introduce 'purgeUnread' config parameter #970
Conversation
Define a new configuration parameter 'purgeUnread' to control whether unread items can be purged. Its default value is 'false'. Extract a method counting items to be removed and introduce another one that ignores if items are still unread. Rearrange 'unread' and 'starred' columns in retrieval and removal queries to make relevant condition optional with a smaller SQL rewrite.
Hey @pefimo do you want to continue to work on this? |
@Grotax I do, but I thought you're not interested since there was no feedback. I'd like to agree on the direction before investing too much time in something that could be rejected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, a lot of feedback happened in #953.
Regarding your code: I would strongly recommend to use the ServiceV2/MapperV2 classes for new features, as the old ones will be removed in the near future.
Also pay attention to the failed checks.
Regarding the functionality of purging unread items: I personally wouldn't use it an my managed instances, but apparently there were some requests for this.
* | ||
* @param string $name Name of the configuration parameter. | ||
*/ | ||
private function readConfigParam($name) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you think it's necessary to have an extra function please also replace the config value for autoPurgeCount
.
Well can't remember that we ever rejected a PR that the author was working on. So far I can't say much, only thin I notice is, we don't want to use manual SQL statements anymore. That's why a lot of the classes got a V2 version upgrading it to new nextcloud and coding standards. I don't think it makes sense to implement something new in the old style. Also unit tests are required of course. Is this code working already, is it deployable? |
Define a new configuration parameter 'purgeUnread' to control whether unread
items can be purged. Its default value is 'false'.
Extract a method counting items to be removed and introduce another one that
ignores if items are still unread.
Rearrange 'unread' and 'starred' columns in retrieval and removal queries to
make relevant condition optional with a smaller SQL rewrite.
TODO: